This is a very weird condition (I fully understand that this is the way it works currently) and a future reader will not understand this. I think it would be helpful to have a NOTE comment or something similar describing why now is used for Infra and thus capture_time is ignored. Not sure if that note should live here or in the code or both.

luckily we no longer need to deal with the nuance of infra vs containers

FYI, existing rollup_spec.rb fails with

     NoMethodError:
       undefined method `utc' for "2010-04-14T21:00:00Z":String
     # ./app/models/vim_performance_state.rb:64:in `capture'
     # ./app/models/metric/ci_mixin/state_finders.rb:34:in `vim_performance_state_for_ts'
     # ./app/models/metric/ci_mixin/state_finders.rb:82:in `containers_from_vim_performance_state_for_ts'
     # ./app/models/metric/rollup.rb:271:in `rollup_child_metrics'
     # ./app/models/metric/rollup.rb:211:in `block in rollup_hourly'
     # ./app/models/metric/rollup.rb:211:in `each'
     # ./app/models/metric/rollup.rb:211:in `rollup_hourly'
     # ./app/models/metric/ci_mixin/rollup.rb:96:in `block (2 levels) in perf_rollup'
     # /Users/joerafaniello/.gem/ruby/3.0.6/gems/more_core_extensions-4.4.0/lib/more_core_extensions/core_ext/benchmark/realtime_store.rb:20:in `realtime_store'
     # /Users/joerafaniello/.gem/ruby/3.0.6/gems/more_core_extensions-4.4.0/lib/more_core_extensions/core_ext/benchmark/realtime_store.rb:56:in `realtime_block'
     # ./app/models/metric/ci_mixin/rollup.rb:95:in `block in perf_rollup'
     # /Users/joerafaniello/.gem/ruby/3.0.6/gems/more_core_extensions-4.4.0/lib/more_core_extensions/core_ext/benchmark/realtime_store.rb:20:in `realtime_store'
     # /Users/joerafaniello/.gem/ruby/3.0.6/gems/more_core_extensions-4.4.0/lib/more_core_extensions/core_ext/benchmark/realtime_store.rb:62:in `realtime_block'
     # ./app/models/metric/ci_mixin/rollup.rb:82:in `perf_rollup'
     # ./spec/models/metric/ci_mixin/rollup_spec.rb:439:in `block (4 levels) in <main>'
     # /Users/joerafaniello/.gem/ruby/3.0.6/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <main>'

It's doing container_image.perf_rollup("2010-04-14T21:00:00Z", 'hourly')

yea. this is a pisser
think it is a legit issue and not just a spec issue.
looking into this.

I went ahead and fixed a caller and added code to protect this.
This code has been moved into 22589 (and this is built upon that)

this is comparing to_i to id, so I did need to keep part of that in there.
But I did use match_array and removed the sort calls.

What are the random numbers for? If it's to get "other" ids, prefer building the id list from existing records, because a) this doesn't handle multi-region properly where the ids aren't in the 0 ragion and b) there's a chance the existing records could fall into the hardcoded range and fail the test.

put in a comment.

these represent ids that are no longer valid.
If container.id is within that range, then one of those ids will work, but the others will be thrown away. so there will be no false positives in here.

I could do containers.last.id and build from there? - but that seemed like a bunch of code that will create invalid ids just as easily as this does.

containers.last.id sounds reasonable - we do this in other specs as well. We've hit problems in other places where we fake ids for records that either don't exist or shouldn't exist and hardcoded ids have always eventually caused problems.

I think we need a few more cases (looking at my drawing that I shared the other day).

(created/deleted wording is relative to the time range in question (i.e. during means during the time range); blank in deleted means not deleted (i.e. still active))

I think we might also need to handle cases where created is nil, which is cases where we're not sure when it was created.

The problem: it is very tricky to define created across all container models.

@Fryguy Do you want to define different active_on scopes for each record?

We don't need created for the first pass as discussed - I think h and i were hard for that reason.

@Fryguy Do you want to define different active_on scopes for each record?

Maybe? Do have a list of the record types that are "different"? Or possibly we can normalize a different way - perhaps @agrare has some thoughts here.

This is the only actual changed line

Why do we have the try here?
-- @Fryguy

If the association is a virtual attribute, like it is for MiqRegion, then it will return an array that does not have a klass.

this was a minor change to return something so the parent knows we need to next
(codeclimate this code to be its own method)